-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tweaks to minimize() behavior, for discussion #217
Conversation
Note that some of the above changes can be argued against ... while the changes above make sense for me, it's not clear where the optimum exactly lies. |
I agree with most of the changes you propose. Catching unknown options is a very good thing in particular. Concerning the new |
Also, there is a few warnings in tests. Did you keep those on purpose? |
21.05.2012 17:22, Denis Laxalde kirjoitti:
Nope. Those should probably be fixed, too. |
Pauli Virtanen a écrit :
Ok. See 26d6d9fccaabd267182990ba80a13818c318f530 in my minimize-opts branch. |
The only point to resolve here seems to be the The approach of Pauli should give reasonable behavior as long as the scales of both x and f(x) are of order 1. If f(x) is O(1) and x is very small, then the difference between Whether it's desirable to then add a |
Now that PR #195 is merged, this one will need a bit of work in order to be merged.
I can spend some time on this but we should coordinate @pv. |
Who has time to finish this PR this weekend? I think this should land in 0.11, because now we can still make changes without having to worry about backwards compatibility. |
I have some time. As far as I see, apart from the last commit (addition of |
Looks like it. The two bullets from Denis above need addressing though, and silencing/fixing those warnings. Would be nice to get |
…nimize_* This simplifies, and probably speeds up the argument unpacking code. Also, check for unknown options, and raise warnings if they are present.
…nce is x-tolerance
Caught by the new unknown options warnings.
Rename option solver parameters: - Add `ftol` to lbfgsb and remove `factr`. - Rename `pgtol` to `gtol` everywhere. - Rename `maxfev` to `maxiter` if the solver did not already have `maxiter`. The aim here is to keep the options as similar as possible across different solvers, preferring to allow slight changes in meaning rather than chaning the option name.
The name `rhoend` is rather confusing for what essentially is a tolerance.
…nused options Fix also unused option warnings in tests.
The plain `ftol` and `xtol` names are used in other routines for relative error, so better keep the nonlin.py solvers in line.
Rebased on top of master, and did the corresponding changes also in |
@@ -18,10 +18,12 @@ | |||
__all__ = ['fmin', 'fmin_powell', 'fmin_bfgs', 'fmin_ncg', 'fmin_cg', | |||
'fminbound', 'brent', 'golden', 'bracket', 'rosen', 'rosen_der', | |||
'rosen_hess', 'rosen_hess_prod', 'brute', 'approx_fprime', | |||
'line_search', 'check_grad', 'Result', 'show_options'] | |||
'line_search', 'check_grad', 'Result', 'show_options', | |||
'OptimizeWarning'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why is OptimizeWarning
in __all__
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It (and other custom Warning classes) are in __all__
because people may want to silence them via warnings.simplefilter("ignore", category=OptimizeWarning)
. Then, it is useful to be able to import it from the public namespace.
Pauli Virtanen a écrit :
It looks good. I'll do a final check and merge afterwards. |
Tweaks to minimize(), minimize_scalar() and root() behavior.
Here's a bunch of tweaks to the minimize() behavior:
tol
parameter to minimize*() functions, for easy tolerance specificationmaxfev
tomaxiter
if the solver does not have amaxiter
parameter. This makes the claim inminimize()
thatmaxiter
is a common parameter for all solvers true.Iterations
are in any case fuzzily defined, so we may as well make this change...pgtol
togtol
rhoend
totol
. It cannot probably be calledxtol
although it is an x-tolerance, as it seems to be an absolute tolerance rather than a relative one.Internal reorganization:
.pop()
approach require an copy of the dictionary).And some bugfixes:
ftol
in brent and golden is actuallyxtol
options
dict for unknown keys is a must --- IIRC, this is something that I disliked also in Matlab's optimization interface.